Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GDScript: Add array support to @export_enum annotation #72951

Closed
wants to merge 1 commit into from

Conversation

dalexeev
Copy link
Member

@dalexeev dalexeev commented Feb 9, 2023

Closes #60516.

@dalexeev dalexeev requested review from a team as code owners February 9, 2023 09:02
@akien-mga akien-mga added this to the 4.x milestone Feb 9, 2023
@akien-mga akien-mga added bug and removed enhancement labels Feb 9, 2023
@akien-mga akien-mga modified the milestones: 4.x, 4.0 Feb 9, 2023
@dalexeev
Copy link
Member Author

dalexeev commented Feb 9, 2023

Probably Array should be allowed too (for ints).

@h0lley
Copy link

h0lley commented Feb 24, 2023

about this closing #60516, there's still the wider issue of @export_* annoations in general having no support for arrays.

for instance, an user may want to export arrays of file paths, bit flags, or ranges just as well.

@export_multiline is currently the sole annotation that already supports arrays, as this is valid: @export_multiline var my_multiline: Array[String].

although if it makes more sense for #60516 to pertain to @export_enum exclusively, I'll change back its title to the original. 🙂

for reference, here's a snippet that can be used to quickly test the array support of all @export_* annoations:

@export_dir var my_dir: Array[String]
@export_file var my_file: Array[String]
@export_global_dir var my_global_dir: Array[String]
@export_global_file var my_global_file: Array[String]
@export_enum("A", "B", "C") var my_enum_str: Array[String]
@export_enum("A", "B", "C") var my_enum_int: Array[int]
@export_flags("A", "B", "C") var my_bit_flag: Array[int]
@export_flags_2d_navigation var my_bit_flag_2d_nav: Array[int]
@export_flags_2d_physics var my_bit_flag_2d_phys: Array[int]
@export_flags_2d_render var my_bit_flag_2d_render: Array[int]
@export_flags_3d_navigation var my_bit_flag_3d_nav: Array[int]
@export_flags_3d_physics var my_bit_flag_3d_phys: Array[int]
@export_flags_3d_render var my_bit_flag_3d_render: Array[int]
@export_multiline var my_multiline: Array[String]
@export_placeholder("Placeholder") var my_placeholder: Array[String]
@export_range(1, 10) var my_range_int: Array[int]
@export_range(1.0, 10.0) var my_range_float: Array[float]
@export_exp_easing var my_exp_easing: Array[float]
@export_node_path var my_node_path: Array[NodePath]
@export_color_no_alpha var my_color: Array[Color]

@dalexeev
Copy link
Member Author

dalexeev commented Feb 24, 2023

See #72912. I'm not sure if we should support all valid combinations of export flags.

I opened this PR because this feature is in 3.x but not in 4.0. This looks like an obvious oversight, because this feature is documented in 3.x, but in other cases it is not so obvious.

Perhaps you are right, and other export annotations should also support arrays.

@h0lley
Copy link

h0lley commented Feb 24, 2023

ah, cool.
although if I understand correctly, @export_custom doesn't add functionality that isn't already achievable with _get_property_list(), no? seems more of an alternative syntax to _get_property_list(), while still not fully replacing it as you cannot add custom logic.

so if I wanted to, say, have an array of PROPERTY_HINT_COLOR_NO_ALPHA appear in the inspector, @export_custom would not help with that.

but I understand that only the missing array support for @export_enum is actually a regression, so #60516 should probably stick to that scope.

@dalexeev
Copy link
Member Author

dalexeev commented Feb 24, 2023

Yes, @export_custom is an "alternative syntax" of _get_property_list(), just like any export annotation. The difference is that this only takes one line and does not require @tool.

You could use @export_custom for this, it's just less convenient (I didn't check if it works):

@export_custom(PROPERTY_HINT_TYPE_STRING, "%s/%s:" % [TYPE_COLOR, PROPERTY_HINT_COLOR_NO_ALPHA])
var array: Array[Color]

But I think other export annotations should also support arrays, thanks for pointing that out. I'll try to expand this PR later or open a separate one (I think it won't require a lot of code). But keep in mind that this will not be included in 4.0.0 anyway.

@dalexeev dalexeev force-pushed the gds-export-enum-array branch 3 times, most recently from aaa03a2 to 4f23534 Compare July 9, 2023 16:42
@@ -329,12 +329,17 @@
<return type="void" />
<param index="0" name="names" type="String" />
<description>
Export an [int] or [String] property as an enumerated list of options. If the property is an [int], then the index of the value is stored, in the same order the values are provided. You can add explicit values using a colon. If the property is a [String], then the value is stored.
See also [constant PROPERTY_HINT_ENUM].
Export an [int], [String], [StringName] or [Array] property as an enumerated list of options (or an array of options). If the property is an [int], then the value is stored. You can add explicit values using a colon. If the property is a [String] or [StringName], then the key is stored. If the property is an [Array] then the array of keys or values is stored (the array type must be [code]Array[/code], [code]Array[int][/code], [code]Array[String] or [code]Array[StringName][/code]).
Copy link
Member Author

@dalexeev dalexeev Jul 9, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your comments are welcome. I feel it's confusing, but I don't know how to explain it in a simpler way.

@dalexeev
Copy link
Member Author

dalexeev commented Oct 7, 2023

@dalexeev dalexeev closed this Oct 7, 2023
@dalexeev dalexeev removed this from the 4.2 milestone Oct 7, 2023
@dalexeev dalexeev deleted the gds-export-enum-array branch October 7, 2023 14:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.